Fix download dialog selector layout#7516
Merged
litetex merged 3 commits intoTeamNewPipe:devfrom Apr 2, 2022
Merged
Conversation
mauriciocolli
added
bug
5 tasks
litetex
requested changes
Dec 8, 2021
Member
litetex
left a comment
litetex
left a comment
There was a problem hiding this comment.
Code looks on the first impression good.
👍 for the tests
However I would appreciate some (there is currently zero) documentation like JavaDoc or comments.
mauriciocolli
force-pushed
the
fix-download-dialog-selector
branch
from
363c37a to
065f9df
Compare
Contributor
Author
|
Done. |
litetex
reviewed
Dec 12, 2021
Member
litetex
left a comment
litetex
left a comment
There was a problem hiding this comment.
LGTM
However I'm unable to get a situation where the icon is visible in the first place.
@mauriciocolli
Do you know a situation/video where that's the case?
Member
There was a problem hiding this comment.
I would rename that method to checkHasAnyVideoOnlyStreamWithNoSecondaryStream
litetex
added
the
waiting for author
litetex
force-pushed
the
fix-download-dialog-selector
branch
from
065f9df to
99d6238
Compare
Member
|
Since there is no feedback for over 2-3 months now (seriously guys stop doing fire-and-forget PRs 😾), I fixed it myself:
|
litetex
removed
the
waiting for author
litetex
approved these changes
Apr 2, 2022
|
Kudos, SonarCloud Quality Gate passed! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is it?
Description of the changes in your PR
Some logic was missing in the adapter for making the no-audio icon
GONEinstead ofINVISIBLE. I fixed it and added some tests as well.That reveals another problem: some icons are not themed properly, I will fix that in another pull request.
Fixes the following issue(s)
Due diligence